Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix remote constants. #7689

Merged
merged 2 commits into from
Nov 9, 2019
Merged

Fix remote constants. #7689

merged 2 commits into from
Nov 9, 2019

Conversation

KochetovNicolai
Copy link
Member

@KochetovNicolai KochetovNicolai commented Nov 8, 2019

I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en

Changelog category (leave one):

  • Bug Fix

Changelog entry (up to few sentences, not needed for non-significant PRs):
Fix possible incorrect result for constant functions from remote servers. It happened for queries with functions like version(), uptime(), etc. which returns different constant values for different servers. #7666

Detailed description (optional):
Get constant values from remote source in RemoteBlockInputStream.

@KochetovNicolai
Copy link
Member Author

Do not know how to create a good simple test.

@akuzm
Copy link
Contributor

akuzm commented Nov 8, 2019

Is this related to #7247 , e.g. should we prevent pushing down non-deterministic functions to remote server?

@KochetovNicolai
Copy link
Member Author

@akuzm I think it is strange to use functions like uptime() and version() in mutations indeed. Result also won't be deterministic. Probably we need another may-be-not-deterministic-on-different-replicas flag?

@akuzm
Copy link
Contributor

akuzm commented Nov 8, 2019

uptime is already marked as non-deterministic, while version is deterministic. Maybe we should mark it as deterministic in scope of query, and only ship deterministic functions to replicas.

@KochetovNicolai
Copy link
Member Author

Yes, it seems reasonable.

@KochetovNicolai KochetovNicolai added the pr-bugfix Pull request with bugfix, not backported by default label Nov 9, 2019
@KochetovNicolai KochetovNicolai merged commit 2233660 into master Nov 9, 2019
KochetovNicolai added a commit that referenced this pull request Nov 9, 2019
Fix remote constants.

(cherry picked from commit 2233660)
KochetovNicolai added a commit that referenced this pull request Nov 9, 2019
Fix remote constants.

(cherry picked from commit 2233660)
KochetovNicolai added a commit that referenced this pull request Nov 9, 2019
Fix remote constants.

(cherry picked from commit 2233660)
KochetovNicolai added a commit that referenced this pull request Nov 9, 2019
Fix remote constants.

(cherry picked from commit 2233660)
alesapin pushed a commit that referenced this pull request Nov 11, 2019
@alesapin alesapin added v19.15 and removed v19.16 labels Nov 11, 2019
@CurtizJ CurtizJ added the v19.17 label Nov 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-bugfix Pull request with bugfix, not backported by default
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants